Code Review

Overview

This PR adds static abstract Info properties to IQuantityOfType<TQuantity> and IQuantity<TSelf, TUnitType> (both #if NET), marks the existing instance QuantityInfo property [Obsolete] on .NET 5+, and introduces GetQuantityInfo() extension methods as the discoverable replacement for callers holding an IQuantity reference.

The design is sound: the instance property always returns a per-type static value, so exposing it as a static abstract eliminates boxing and virtual dispatch.

Breaking Changes

Soft break (intentional): IQuantity.QuantityInfo and the typed variants are now [Obsolete] on .NET 5+. Existing callers will see CS0618 warnings. This is the right call.

Potential hard break for custom quantities: GetUnitInfo(), GetUnitInfo<TQuantity, TUnit>(), and As<TQuantity>(UnitSystem) now delegate to GetQuantityInfo(), which looks up the type in UnitsNetSetup.Default.Quantities. Custom IQuantity implementations not registered there will now throw where they previously returned the correct info. This is a behaviour change for library consumers who implement custom quantities and use these extensions.

Consider a note in the XML docs for GetQuantityInfo() and a CHANGELOG entry.

Code Quality

Positives:

• The default static implementation static QuantityInfo IQuantityOfType<TSelf>.Info => TSelf.Info; in IQuantity<TSelf, TUnitType> is elegant -- existing implementors of the typed interface automatically satisfy the non-generic requirement without code changes.
• All #pragma warning disable CS0618 suppressions are accompanied by comments explaining why each suppression is necessary. This is exactly the right pattern.
• The #if NET / #else fallbacks in LinearQuantityExtensions, LogarithmicQuantityExtensions, and QuantityExtensions correctly use TQuantity.From(...) on modern targets and the instance-based path on netstandard2.0.

Suggestions:

1. GetQuantityInfo() XML doc -- document the throw: The current remark says "looked up via UnitsNetSetup.Default" but does not mention it throws for unregistered types. An <exception> element would prevent surprises for users with custom quantities.

2. QuantityTypeConverter -- micro-opportunity on .NET 5+: default(TQuantity).QuantityInfo.Name constructs a dummy struct just to reach a static value. The constraint where TQuantity : struct, IQuantity does not include IQuantityOfType<TQuantity>, so TQuantity.Info is not reachable without widening it. Widening is a separate refactor, so the current #pragma approach is fine -- just noting it as a follow-up.

3. Benchmark: Changing quantity.QuantityInfo.UnitInfos to quantity.GetQuantityInfo().UnitInfos adds a registry dictionary lookup per outer iteration. Since the benchmark measures quantity.As(unitInfo.Value) in the inner loop this is negligible, but if benchmark purity matters, caching the info outside the outer loop would be cleaner.

Test Coverage

The four new tests in IQuantityTests.cs are well-chosen:

• Round-trip between Mass.Info, non-generic GetQuantityInfo(), and typed GetQuantityInfo().
• Static abstract access via both IQuantityOfType<T> and IQuantity<TSelf, TUnit> generic constraints.

Nice-to-have: a test covering the throw behaviour of GetQuantityInfo() / GetUnitInfo() when called on an unregistered custom quantity (the HowMuch type in the test project would be a natural candidate). This would document the breaking behaviour and prevent silent regressions.

Minor Nit

The diff removes the UTF-8 BOM from line 1 of IQuantity.cs. Harmless, but unrelated to the PR.

Summary

Overall a well-designed change with a clear rationale and solid backward-compatibility strategy. The main concern worth addressing before merge is explicitly documenting (and testing) that GetQuantityInfo() / GetUnitInfo() will throw for unregistered custom quantities, since that is a behavioural change from the previous instance-property path.

Code Review

Overview

This PR adds static abstract Info members to IQuantityOfType<TQuantity> and IQuantity<TSelf,TUnitType>, marks the instance QuantityInfo property as [Obsolete] on .NET 5+, and adds GetQuantityInfo() extension methods as the new discoverable API for interface-typed references. The design rationale is solid — instance QuantityInfo returning a per-type static value through interface dispatch (with boxing on structs) is a well-known antipattern.

Breaking Changes

Obsolete QuantityInfo property

Marking IQuantity.QuantityInfo as [Obsolete] on .NET 5+ is a source-level breaking change for any downstream project that sets <TreatWarningsAsErrors>true</TreatWarningsAsErrors> (which is common in modern .NET projects). This deserves a note in the release notes and the PR description.

Custom quantity authors

The HowMuch test change shows a required migration: custom quantities using public static readonly QuantityInfo<T, TUnit> Info = new(...) (a field) must change to public static QuantityInfo<T, TUnit> Info { get; } = new(...) (a property) to satisfy the new static abstract interface. This affects anyone implementing the interface outside this repository and should be called out in the changelog/migration guide.

Potential Issues

GetQuantityInfo() silently breaks for unregistered custom quantities

The extension method routes through UnitsNetSetup.Default.Quantities.GetQuantityInfo(quantity.GetType()), which throws for any custom quantity not registered there. The PR handles this correctly in the few places that still use the instance property (serializers, debugger proxy, QuantityTypeConverter), but the documentation on the new extension method only says "looked up via UnitsNetSetup.Default". Consider adding an explicit <exception> doc tag (e.g. <exception cref="KeyNotFoundException">) or a <remarks> note to warn callers:

/// <remarks>
///     This method throws if the quantity's runtime type is not registered in
///     <see cref="UnitsNetSetup.Default"/>. Custom quantities must be registered
///     first via <see cref="UnitsNetSetup"/> or use the instance <c>QuantityInfo</c>
///     property directly.
/// </remarks>

Typed cast can silently mismatch

public static QuantityInfo<TUnit> GetQuantityInfo<TUnit>(this IQuantity<TUnit> quantity)
    => (QuantityInfo<TUnit>)UnitsNetSetup.Default.Quantities.GetQuantityInfo(quantity.GetType());

If the registered QuantityInfo does not match TUnit, this throws an InvalidCastException with no useful message. A guarded cast with a descriptive error would help diagnostics.

Benchmark now measures dictionary lookup

In QuantityConversionBenchmarks.cs, the hot loop changed from quantity.QuantityInfo (a vtable call caching aside) to quantity.GetQuantityInfo() (dictionary lookup per call). This alters what the benchmark measures. Since the benchmark works with IQuantity references and can't use TSelf.Info directly, consider caching the result before the inner loop, or adding a comment noting the intentional change.

Test Coverage

The four new tests in IQuantityTests.cs are well-structured and test the important round-trip equivalences. One gap:

• No test verifies that GetQuantityInfo() throws an appropriate exception for a custom quantity not registered in UnitsNetSetup.Default (e.g. HowMuch). Given that several call sites kept the old instance property specifically for this reason, a test documenting the expected behaviour would be valuable.

Minor / Style

• The BOM removal from IQuantity.cs is a harmless housekeeping fix.
• The #pragma warning disable/restore pairs are well-commented with explanations — good pattern to keep.
• LinearQuantityExtensions, LogarithmicQuantityExtensions, and QuantityExtensions all follow the same #if NET / #else / #endif pattern consistently. Looks clean.

Summary

The overall direction is correct and the implementation is careful. The main things to address before merging:

1. Document (changelog / migration guide) that [Obsolete] is a source break for TreatWarningsAsErrors users and that custom quantity fields must become properties.
2. Improve GetQuantityInfo() XML docs to call out the "not-registered" exception path.
3. Consider fixing the benchmark to not inadvertently measure the new dictionary-lookup overhead.

Code Review — PR #1675: Add static abstract Info and obsolete QuantityInfo instance member on interfaces

Overview

This PR introduces static abstract Info on IQuantityOfType<TQuantity> and IQuantity<TSelf, TUnitType> under #if NET, marks the instance QuantityInfo property as [Obsolete] on .NET 5+, and adds GetQuantityInfo() extension methods as the recommended replacement for dynamic callers. The motivation (eliminating boxing and virtual dispatch in generic algorithms) is sound and the implementation is clean.

Potential Issues

1. Behavioral regression for unregistered custom quantities (medium severity)

GetUnitInfo() and As(UnitSystem) were migrated to call GetQuantityInfo(), which routes through UnitsNetSetup.Default. QuantityInfoLookup.GetQuantityInfo(Type) throws QuantityNotFoundException if the type isn't registered. On .NET 5+ these methods now throw where before they silently worked:

// QuantityExtensions.cs — was: quantity.QuantityInfo[quantity.UnitKey]
return quantity.GetQuantityInfo()[quantity.UnitKey]; // throws for unregistered custom quantities

The PR correctly suppresses the obsolete warning with an explanatory comment in serialization/debug proxy code for exactly this reason, but the same consideration wasn't applied to GetUnitInfo() or As(UnitSystem).

For the generic overload GetUnitInfo<TQuantity, TUnit>(), TQuantity is already constrained to IQuantity<TQuantity, TUnit>, so TQuantity.Info is available on .NET 5+ — no registry lookup needed:

#if NET
    return TQuantity.Info[quantity.Unit];
#else
    return quantity.QuantityInfo[quantity.Unit];
#endif

2. Build-breaking for callers with <TreatWarningsAsErrors>true</TreatWarningsAsErrors>

The new [Obsolete] attributes on IQuantity.QuantityInfo will turn into compile errors for downstream projects that treat warnings as errors. This is a real upgrade-path concern and should be explicitly called out in the release notes or migration guide.

3. Benchmark now measures registry lookup overhead

// QuantityConversionBenchmarks.cs
foreach (UnitInfo unitInfo in quantity.GetQuantityInfo().UnitInfos)  // was: quantity.QuantityInfo.UnitInfos

This adds a dictionary lookup to what was previously a pure conversion benchmark, changing what's being measured. Either revert this file or add a separate benchmark that isolates the two costs.

Strengths

• Clean delegation pattern: static QuantityInfo IQuantityOfType<TSelf>.Info => TSelf.Info in IQuantity<TSelf, TUnitType> satisfies both interface members with a single implementation in generated code — no codegen change required.
• Pragma comments are explicit and honest about why the obsolete member must remain in serialization, debug proxy, and QuantityTypeConverter code.
• Good test coverage — all four API paths are verified (non-generic extension, generic extension, both static abstract members via constrained generics).
• netstandard2.0 contract is preserved: #if NET guards are used consistently throughout.
• The HowMuch custom quantity change from public static readonly field to public static … { get; } property is the correct fix to satisfy the static abstract contract.

Minor

• The GetQuantityInfo<TUnit>() extension casts the result: (QuantityInfo<TUnit>)…GetQuantityInfo(quantity.GetType()). This is safe for all built-in quantities, but will throw InvalidCastException if a custom quantity registers its info under a different unit type than TUnit. Low risk, but worth documenting in the XML doc.

Summary

The design is well thought-out and the implementation is mostly correct. The main asks before merging:

1. Fix GetUnitInfo<TQuantity, TUnit> to use TQuantity.Info on .NET 5+ instead of GetQuantityInfo() to avoid the registry dependency for unregistered custom quantities.
2. Decide whether GetUnitInfo() (non-generic) and As(UnitSystem) should document that they require registration in UnitsNetSetup.Default on .NET 5+, or add a fallback.
3. Revert the benchmark change or isolate the registry-lookup cost into a separate benchmark.

Code Review: Add static abstract Info and obsolete QuantityInfo instance member on interfaces

Summary: This PR adds static abstract Info to IQuantityOfType and IQuantity, marks the instance QuantityInfo property Obsolete on .NET 5+, and introduces GetQuantityInfo() extension methods as the new discoverable API. The design is clean and well-reasoned.

Breaking Changes

1. CS0618 warnings for existing callers (source break on .NET 5+)
   Marking IQuantity.QuantityInfo and its overrides [Obsolete] will produce warnings for every caller on .NET 5+. Callers who compile with TreatWarningsAsErrors=true (common in libraries) will see build failures. This should be noted in a migration guide or CHANGELOG.

2. Behavioral difference: GetQuantityInfo() vs QuantityInfo for unregistered quantities
   The new GetQuantityInfo() goes through UnitsNetSetup.Default.Quantities. Users migrating from quantity.QuantityInfo to quantity.GetQuantityInfo() will get a runtime KeyNotFoundException for any custom quantity not registered in UnitsNetSetup.Default. The PR handles known internal sites with pragma warning disable and explains why -- but the GetQuantityInfo() XML doc should mention this failure mode.

Performance

1. Benchmark hot path regression: QuantityConversionBenchmarks.cs changed from quantity.QuantityInfo.UnitInfos to quantity.GetQuantityInfo().UnitInfos. GetQuantityInfo() adds a GetType()-keyed dictionary lookup per iteration that adds measurement noise. Consider keeping pragma warning disable CS0618 here to preserve the benchmark's intent.

2. Positive: hot generic paths now use TQuantity.From(...) and TQuantity.Info instead of instance dispatch, eliminating boxing for struct quantities in tight loops.

Code Quality

1. GetQuantityInfo typed overload does an unchecked cast. If called with a mismatched unit type, the lookup succeeds but throws InvalidCastException. Fine in practice but a cleaner error would help debugging.

2. The pragma warning disable CS0618 blocks are all well-documented with clear rationale.

3. The UTF-8 BOM was stripped from IQuantity.cs -- minor inconsistency worth checking against other files.

Tests

The four new tests cover non-generic extension, typed extension, and both static abstract paths. Assert.Same confirms singleton behavior.

Missing: a test for calling GetQuantityInfo() on an unregistered custom quantity to document the expected failure.

Minor

QuantityExtensions.As(UnitSystem) diverges between NET (uses GetQuantityInfo, fails for unregistered) and netstandard2.0 (uses instance property, always works). A comment explaining the asymmetry would help.

HowMuch.Info field-to-property change is correct and necessary.

Verdict

The design is sound and the implementation is careful. Main asks before merging:

1. Document the KeyNotFoundException failure mode on GetQuantityInfo() and make the custom-quantity caveat more prominent.
2. Revert the benchmark to use pragma warning disable CS0618 so it measures conversion overhead, not registry lookup overhead.
3. Note the CS0618 source break in CHANGELOG for library consumers using TreatWarningsAsErrors.

Code Review -- PR #1675: Static abstract Info + obsolete instance QuantityInfo

Overview

This PR addresses a real design smell: IQuantity.QuantityInfo is an instance property that always returns the same per-type static value, implying variance it does not have and forcing interface-dispatch (boxing on structs) on every call. Moving to a static abstract member on the interfaces and adding GetQuantityInfo() extension methods as the discoverable replacement for non-generic call-sites is the right approach.

Breaking changes

• Adding [Obsolete] to IQuantity.QuantityInfo on .NET 5+ is a source-level soft breaking change (warning, not error). Callers that compile with warnings-as-errors will see build failures. The migration path (TSelf.Info or GetQuantityInfo()) is clear, and the netstandard2.0 contract is preserved.
• Behavioral difference introduced silently in some extension methods: GetUnitInfo() and As(UnitSystem) now route through UnitsNetSetup.Default.Quantities on .NET (via GetQuantityInfo()), but previously used the direct instance property. Any custom quantity not registered in UnitsNetSetup.Default will now throw from these methods on .NET 5+, while they worked unconditionally before. The serialization and debug-proxy sites get explicit #pragma comments explaining why they keep the instance property -- the migrated extension-method call-sites deserve the same treatment or a note in the GetQuantityInfo() XML doc flagging this new constraint.

Design concerns

GetQuantityInfo() silently differs from the replaced property for unregistered quantities

The new extension method requires the quantity to be registered in UnitsNetSetup.Default. The obsoleted property worked for any quantity, registered or not. Several sites migrated to GetQuantityInfo() without any comment explaining the new constraint (e.g. GetUnitInfo(), As(UnitSystem)). Suggest adding to the XML doc: "Throws KeyNotFoundException if the quantity's runtime type is not registered in UnitsNetSetup.Default."

Missing fully-typed overload

There are two overloads so far:

• GetQuantityInfo(this IQuantity) -> QuantityInfo
• GetQuantityInfo<TUnit>(this IQuantity<TUnit>) -> QuantityInfo<TUnit>

Callers holding an IQuantity<TSelf, TUnit> reference get back QuantityInfo<TUnit>, not the fully-typed QuantityInfo<TSelf, TUnit>. On .NET they can use TSelf.Info directly, so this is low priority -- could be a follow-up.

Benchmark now measures registry overhead

QuantityConversionBenchmarks.cs changed from quantity.QuantityInfo (direct property) to quantity.GetQuantityInfo() (dictionary lookup). The benchmark now inadvertently measures registry lookup overhead alongside conversion cost, muddying the baseline. Consider keeping the benchmark on the instance property (with a #pragma) or add a comment that the numbers now include lookup cost.

Good practices

• The default static implementation static QuantityInfo IQuantityOfType<TSelf>.Info => TSelf.Info; avoids redundant overrides in all 130+ generated quantity types -- no codegen change needed. Clean.
• Replacing firstQuantity.QuantityInfo.From(...) with TQuantity.From(...) in LinearQuantityExtensions and LogarithmicQuantityExtensions is a nice improvement: it removes the dependency on a first-element reference and is clearer in intent.
• Consistent #pragma warning disable CS0618 with clear explanatory comments at sites that legitimately keep the instance property (serialization, debug proxy, type converter). Well done.
• Tests verify same-instance identity via Assert.Same -- the right assertion. Consider adding a test verifying that GetQuantityInfo() throws for a custom quantity not registered in UnitsNetSetup.Default (e.g. a bare HowMuch before registration), to document and lock in the behavior.

Minor nit

The diff shows a stray UTF-8 BOM (U+FEFF) removal on line 1 of IQuantity.cs. If unintentional, revert to keep the diff focused.

Code Review

Overview

This PR introduces static abstract Info members on IQuantityOfType<TQuantity> and IQuantity<TSelf, TUnitType> (.NET 5+ only), marks the instance QuantityInfo property [Obsolete] on .NET 5+, and adds GetQuantityInfo() extension methods as the discoverable replacement for code holding only an IQuantity reference.

The design rationale is solid: the instance property returns a per-type static value through interface dispatch, including boxing on structs. The default static bridge static QuantityInfo IQuantityOfType<TSelf>.Info => TSelf.Info; in IQuantity<TSelf, TUnitType> is elegant -- existing typed implementors automatically satisfy the non-generic interface with no code changes.

Breaking Changes

Soft break (obsolete warning): IQuantity.QuantityInfo and the typed variants become [Obsolete] on .NET 5+. For any downstream project with <TreatWarningsAsErrors>true</TreatWarningsAsErrors> (common in modern .NET), this is an effective hard break. Worth a CHANGELOG entry and migration note.

Custom quantity implementors -- field to property: The HowMuch change shows a required migration for anyone implementing the static abstract interface:

// Before (field -- does not satisfy static abstract)
public static readonly QuantityInfo<HowMuch, HowMuchUnit> Info = new(...);

// After (property -- satisfies static abstract)
public static QuantityInfo<HowMuch, HowMuchUnit> Info { get; } = new(...);

This is a compile-time break for external custom quantities. The migration guide / release notes should call this out explicitly.

Potential Issues

1. GetQuantityInfo() throws for unregistered custom quantities

public static QuantityInfo GetQuantityInfo(this IQuantity quantity)
    => UnitsNetSetup.Default.Quantities.GetQuantityInfo(quantity.GetType());

The PR correctly keeps #pragma warning disable CS0618 in the serialisers, debugger proxy, and QuantityTypeConverter for exactly this reason -- those paths must support unregistered custom quantities. However, the extension method's XML doc only mentions it is "looked up via UnitsNetSetup.Default" without stating that it throws for unregistered types. Callers who migrate from quantity.QuantityInfo to quantity.GetQuantityInfo() may hit this at runtime with no warning.

A short <remarks> or <exception cref="KeyNotFoundException"> tag on the extension method would prevent surprises.

2. Typed overload unchecked cast

public static QuantityInfo<TUnit> GetQuantityInfo<TUnit>(this IQuantity<TUnit> quantity)
    => (QuantityInfo<TUnit>)UnitsNetSetup.Default.Quantities.GetQuantityInfo(quantity.GetType());

A mismatch produces an InvalidCastException with no diagnostic message. A guarded cast with a descriptive error message (similar to patterns elsewhere in the codebase) would make failures actionable.

Code Quality -- Positives

• All #pragma warning disable CS0618 suppressions carry a comment explaining why the instance property is still needed in that specific spot. Exactly the right pattern.
• The #if NET / #else blocks in LinearQuantityExtensions, LogarithmicQuantityExtensions, and QuantityExtensions cleanly adopt TQuantity.From(...) on modern targets without regressions on netstandard2.0.

Test Coverage

The four new tests in IQuantityTests.cs cover the important surface:

• Non-generic GetQuantityInfo() returns same instance as Mass.Info.
• Typed GetQuantityInfo<TUnit>() returns same instance.
• Static abstract IQuantity<TSelf, TUnit>.Info access via generic constraint.
• Static abstract IQuantityOfType<T>.Info access via generic constraint.

One gap worth adding: a test for GetQuantityInfo() called on an unregistered custom quantity (e.g. HowMuch) to document and guard the throw behaviour.

Minor Nit

The diff removes the UTF-8 BOM from the first line of IQuantity.cs. Harmless but unrelated to the PR; best reverted to keep the diff focused.

Summary

Well-designed change with a clear rationale and a sensible backward-compatibility strategy. Main items to address before merging:

1. Document (and test) that GetQuantityInfo() throws for unregistered custom quantities -- this is a behavioural change from the previous instance-property path.
2. Add a migration note for custom quantity authors who must change static readonly fields to static properties.
3. Consider a guarded cast with a descriptive error on the typed GetQuantityInfo<TUnit>() overload.
4. Revert the incidental BOM removal.